Pull request overview

This PR adds an opt-in multi-layer routing overlay for Pyramid hierarchy roots (HGraph-style sparse route layers + existing complete bottom graph) and aligns Pyramid’s search-time reorder behavior with the common factor parameter, including additional stats and serialization support.

Changes:

• Add per-hierarchy root_graph_type (single_layer default, multi_layer opt-in) and implement root route-graph build/search + persistence.
• Apply factor to cap reorder candidate count (while preserving requested final TopK) and expose reorder_candidate_count in query statistics.
• Enforce hops_limit in ParallelSearcher and extend Pyramid’s hop limiting behavior to non-root graph nodes (while excluding route graphs).

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

    bool has_index = false;
    {
        std::shared_lock lock(mutex_);
        has_index = status_ != IndexNode::Status::NO_INDEX;
    }

                    std::unique_lock route_lock(h_ptr->root_routing_mutex);
                    if (not h_ptr->root_routes_initialized) {
                        std::shared_lock root_lock(h_ptr->root->mutex_);
                        rebuild_root_routes_by_nsw(*h_ptr);
                        h_ptr->root_routes_initialized = true;

    for (int route_level = current_top + 1; route_level <= level; ++route_level) {
        auto graph = make_root_route_graph(hierarchy);
        graph->InsertNeighborsById(inner_id, Vector<InnerIdType>(allocator_));
        hierarchy.root_route_graphs.push_back(std::move(graph));
        hierarchy.root->entry_point_ = inner_id;
    }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

        search_result = this->reorder_->Reorder(search_result,
                                                query->GetFloat32Vectors(),
                                                search_param.topk,
                                                ctx,

| `factor` | float | unset | Reorder candidate multiplier. When set to `<= 1`, reorder up to `max(ef_search, topk)` candidates; when greater than `1`, reorder up to `min(max(ef_search, topk), floor(topk * factor))`. It must be finite and positive. It has no effect when reorder is disabled. |

| `factor` | float | 未设置 | 重排候选倍率。值 `<= 1` 时最多重排 `max(ef_search, topk)` 个候选；值大于 `1` 时最多重排 `min(max(ef_search, topk), floor(topk * factor))` 个候选。必须为有限正数；关闭重排时不生效。 |

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

    if (json.Contains(PYRAMID_ROOT_GRAPH_TYPE)) {
        CHECK_ARGUMENT(json[PYRAMID_ROOT_GRAPH_TYPE].IsString(),
                       "root_graph_type must be a string");
        this->root_graph_type = json[PYRAMID_ROOT_GRAPH_TYPE].GetString();
    }
    validate_root_graph_config(this->root_graph_type, this->no_build_levels, "Pyramid");

                                           node->entry_point_);

            deserialize_root_routes(buffer_reader, *h_iter->second);

    if (json.Contains(PYRAMID_ROOT_GRAPH_TYPE)) {
        CHECK_ARGUMENT(json[PYRAMID_ROOT_GRAPH_TYPE].IsString(),
                       fmt::format("hierarchy {} root_graph_type must be a string", name));
        root_graph_type = json[PYRAMID_ROOT_GRAPH_TYPE].GetString();
    }

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

        search_result = this->reorder_->Reorder(search_result,
                                                query->GetFloat32Vectors(),
                                                search_param.topk,
                                                ctx,
                                                nullptr,
                                                rabitq_lower_bound_candidates);

        if (json.Contains(SEARCH_PARAM_FACTOR)) {
            topk_factor = json[SEARCH_PARAM_FACTOR].GetFloat();
            has_topk_factor = true;
        }

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

            "precise_quantization_type": "{}",
            "use_reorder": {},
            "index_min_size": 28{},
            "support_duplicate": {}
        }}
    }}
    )";
    const auto root_graph_parameter =
        param.root_graph_type.has_value()
            ? fmt::format(",\n            \"root_graph_type\": \"{}\"", *param.root_graph_type)
            : "";

                       "precise codes capacity is smaller than the encoded id range");
    }
    if (raw_vector_ != nullptr) {
        CHECK_ARGUMENT(raw_vector_->max_capacity_ >= required_capacity,
                       "raw vector capacity is smaller than the encoded id range");
    }

    const auto* data_vectors = base->GetFloat32Vectors();
    const auto encode_range = [this, data_vectors, &batch](uint64_t begin, uint64_t end) {
        for (uint64_t offset = begin; offset < end; ++offset) {
            const auto* vector = data_vectors + dim_ * batch.input_indices[offset];
            const auto inner_id = static_cast<InnerIdType>(batch.first_inner_id + offset);
            base_codes_->InsertVector(vector, inner_id);
            if (has_precise_reorder()) {
                precise_codes_->InsertVector(vector, inner_id);
            }
            if (raw_vector_ != nullptr) {
                raw_vector_->InsertVector(vector, inner_id);

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

    if (json.Contains(PYRAMID_ROOT_GRAPH_TYPE)) {
        CHECK_ARGUMENT(json[PYRAMID_ROOT_GRAPH_TYPE].IsString(),
                       "root_graph_type must be a string");
        this->root_graph_type = json[PYRAMID_ROOT_GRAPH_TYPE].GetString();
    }

Code Review for PR #2717

[critical] IndexNode::Search children_ data race regression

File: src/algorithm/pyramid/pyramid.cpp, IndexNode::Search() (line ~746 in new file)

Commit bc31f9cf added a snapshot of child pointers under shared_lock before iterating, which addressed the data race between concurrent Add (creating new path nodes via AddChild) and Search (traversing children_). Commit 81535481 ("refactor(pyramid): simplify routed build lifecycle") removed the snapshot and reverted to direct iteration of children_ without the lock.

The children_ map (UnorderedMap<std::string, std::shared_ptr<IndexNode>>) is mutated by GetChild/AddChild under node.mutex_. Concurrent Search iterating the map without synchronization is a data race and undefined behavior.

Please restore the snapshot pattern or hold a shared_lock for the duration of the iteration.

[suggestion] build_with_cache does not seed route graphs from cache

File: src/algorithm/pyramid/pyramid.cpp, build_with_cache() (~line 2755)

When build_with_cache processes a node with has_routing() == true, cache-hit entries seed only the bottom graph (gnode->graph_->InsertNeighborsById). The route graphs are built from scratch during the subsequent refinement phase via add_routed_point. This means the route graph structure is not preserved across rebuilds, which could lead to different search performance characteristics after a cache-based rebuild compared to a fresh build.

Consider whether route graph neighbor lists should also be cached and restored, or document this as an intentional design trade-off.

[note] Overall assessment

The PR is well-structured with thorough test coverage (9 new test cases covering build, serialization, concurrent Add/Search, factor control, duplicate handling, hops_limit, and parameter validation). The double-checked locking pattern in add_routed_point, the seed-first parallel insertion strategy, and the dedicated random_generator_mutex_ all demonstrate careful attention to concurrency correctness. The serialization compatibility checks and parameter validation are comprehensive.

One regression noted above (children_ snapshot removal) should be addressed before merge.

Code Review for PR #2717 (commit 14e016c)

Summary

This PR adds multi-layer root routing to the Pyramid index, enabling sparse route graphs above the dense Flat bottom graph. The architecture is well-structured with clear separation between routing and bottom graph operations. The double-checked locking pattern for routing graph growth is correct, and the seed-first insertion strategy ensures the routing structure is established before parallel workers run.

Suggestions

[suggestion] add_routed_point double-checked locking: TOCTOU window between unlock and relock
src/algorithm/pyramid/pyramid.cpp lines 411-412: After releasing the write lock and before re-acquiring the shared lock, the routing structure could theoretically be modified by another thread. While benign in practice (routing graphs only grow via push_back, and insert_locked() recomputes current_top), the pattern is fragile. Consider adding a comment explaining the safety invariant.

[suggestion] entry_point_ read inside insert_locked under shared_lock
src/algorithm/pyramid/pyramid.cpp line 317: node.entry_point_ is read inside insert_locked(), which may execute under a shared_lock. This is safe because the seed insertion (which writes entry_point_) always runs under unique_lock before any worker runs under shared_lock. However, this depends on the invariant that the seed has the maximum sampled level. If the seed selection logic changes in the future, this could become a data race. Consider adding a comment documenting this invariant.

[suggestion] build_with_cache double-seed pattern for hit_ids vs missed_ids
src/algorithm/pyramid/pyramid.cpp lines 2805-2808: refine_nodes is called separately for node_missed_ids and node_hit_ids, each with its own run_parallel_insertions call. This means two seeds are inserted (one for missed, one for hit). If the hit seed has a higher level than the miss seed, the routing graphs need to grow during the hit phase, which is handled correctly by the double-checked locking. However, the interaction between the two phases is subtle. Consider adding a brief comment explaining that the second seed may trigger routing graph growth.

[note] hops_limit=10000 hardcoded in add_routed_point and add_bottom_graph_point
src/algorithm/pyramid/pyramid.cpp lines 338 and 2045: The construction-time graph search uses a hardcoded hops_limit=10000. This is consistent with the existing add_bottom_graph_point behavior, but differs from the search path where hops_limit is configurable. If very large graphs are built, this limit could be reached. Consider making this configurable or documenting the rationale.

What was verified

• Multi-layer routing architecture: add_routed_point → route graph traversal → bottom graph insertion → route graph insertion
• Double-checked locking for routing graph growth is correct
• Seed-first insertion ensures routing structure exists before parallel workers
• resolve_entry_point correctly traverses route graphs top-down during search
• Serialization/deserialization of routing graphs with MAX_ROOT_ROUTE_GRAPH_COUNT=1024 validation
• build_with_cache correctly handles routed root nodes
• FLAT-to-GRAPH promotion does not incorrectly enable routing for non-root nodes
• entry_point_ updates are properly protected by node.mutex_

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

            "fast_encode_rabitq": {},
            "fast_encode_rabitq_rounds": 6,
            "precise_quantization_type": "{}",
            "use_reorder": {},
            "index_min_size": 28{},
            "support_duplicate": {}
        }}
    }}
    )";
    const auto root_graph_parameter =
        param.root_graph_type.has_value()
            ? fmt::format(",\n            \"root_graph_type\": \"{}\"", *param.root_graph_type)
            : "";

validate_root_graph_config(const std::string& root_graph_type,
                           const std::vector<int32_t>& no_build_levels,
                           int64_t max_degree,
                           const std::string& context) {
    validate_root_graph_type(root_graph_type, context);
    CHECK_ARGUMENT(
        root_graph_type != PYRAMID_ROOT_GRAPH_TYPE_MULTI_LAYER ||
            std::find(no_build_levels.begin(), no_build_levels.end(), 0) == no_build_levels.end(),
        fmt::format("{} multi-layer root graph requires level 0 to be built", context));
    CHECK_ARGUMENT(
        root_graph_type != PYRAMID_ROOT_GRAPH_TYPE_MULTI_LAYER || max_degree > 1,
        fmt::format("{} multi-layer root graph requires max_degree greater than 1", context));
}